fix(node): implement reconciliation sweep as durability backstop (#218) - #244

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The security design here is genuinely careful, and I want to lead with that: I could not construct any input (rule shape, is_public value, quarantine timing, or DID form) that makes the sweep pin, announce, seal-in-plaintext, or anchor content a private repo must withhold. The announceable gate evaluates the anonymous perspective (listable_at_root(..., None), so the owner short-circuit never fires), the object filter is the anon-perspective fail-closed set, all four sinks run only on that filtered set, the encrypted phase seals ciphertext, and quarantine is rechecked before pinning and fails closed on error. That is the hard part and it is done well.

The durability mechanics are where the problems are: one hard break plus several coverage/cost holes that undercut the guarantee the PR is written to provide. Findings highest first.

Findings

• [P1] Drop the pinned_cids.cid NOT NULL constraint before writing NULL Pinata-only rows
  crates/gitlawb-node/src/db/mod.rs:2342
record_pinata_cid now binds cid = NULL for new rows, but the column is cid TEXT NOT NULL and no migration relaxes it. Every first-time Pinata pin fails the INSERT with a NOT NULL violation — this is not sweep-only, it globally breaks the Pinata write path (the push-time pin calls the same function), so Pinata-only state never records and the caller retries into the same error. Main binds cid = pinata_cid, so this is a regression introduced here. Ship a new migration that does ALTER TABLE pinned_cids ALTER COLUMN cid DROP NOT NULL (and reconcile it with main's existing pinata_cid work, see the stale-base note below). Reproduce with a fresh object, Pinata configured, IPFS unconfigured: the insert errors and has_pinata_cid stays false.

• [P2] Subtract already-pinned objects before the per-repo cap, or page within the repo
  crates/gitlawb-node/src/reconciliation.rs:181
object_list is truncated to MAX_OBJECTS_PER_REPO (50k) before the IPFS/Pinata missing-set is computed. On a stable list_all_objects order, a repo with more than 50k replicable objects always presents the same prefix; if that prefix is already pinned and the dropped object sits past it, the gap is never a candidate and the sweep reports success while the hole persists — exactly the large-history case the backstop exists for. Compute the missing set first (or page the scan) so coverage does not stop at the cap.

• [P2] Order the sweep cursor by a stable key so idle repos are not starved
  crates/gitlawb-node/src/reconciliation.rs:99
  The cursor is a positional index into list_all_repos_deduped(), which is ORDER BY updated_at DESC. Every push reshuffles that order, so hot repos cluster at low indices while cold/idle repos drift around the cursor and can be skipped indefinitely — and idle repos are precisely the ones with only the sweep as a safety net. Order the eligible set by a stable key (id or created_at) so the positional cursor deterministically covers everyone.

• [P2] Bound the object walk itself, not only the post-walk pin batch
  crates/gitlawb-node/src/reconciliation.rs:142
list_all_objects runs git cat-file --batch-all-objects and materializes one String per object with no streaming, before MAX_OBJECTS_PER_REPO applies. A repo with millions of loose objects spikes ~1GB transient on one blocking thread per pass; since repos are sequential, one pathological repo stalls the rest of that pass. The comment at the top of the file claims the cap prevents monopolizing the blocking pool, but the cap bounds pin work, not scan cost.

• [P2] Do not re-anchor the full encrypted manifest to Arweave every pass
  crates/gitlawb-node/src/reconciliation.rs:323
  Phase 2 anchors the whole merged manifest for any path-scoped repo that has any encrypted_blobs row, on every hourly pass, even when encrypt_and_pin sealed nothing new. That is a paid permanent-ledger write on a timer; a caller who creates public path-scoped repos with withheld blobs turns one-time sealing into unbounded anchor spend. Gate the anchor on "something new was sealed this pass, or the last anchor is known to have failed."

• [P2] Rebase off the 36-commit-stale base and re-review the merged state
  crates/gitlawb-node/src/db/mod.rs:2159
  The base is 36 commits behind main and both touch db/mod.rs. This PR removes is_pinned, changes record_pinned_cid's ON CONFLICT from DO NOTHING to DO UPDATE, and introduces a cid = NULL Pinata convention, while main independently evolved the same pinned_cids/pinata_cid area (it kept is_pinned with a live caller and added has_pinata_cid rather than this PR's has_ipfs_cid). The shipped behavior is the rebase resolution, not what the diff shows, so this needs a rebase and a re-review on merged state before it can land.

• [P2] Add tests for the leak-class and coverage-critical behavior
  crates/gitlawb-node/src/reconciliation.rs:1
  The diff ships no tests. For a feature that emits repo content to public networks under a visibility filter, the fail-closed properties and the coverage guarantee need guards: a private repo produces zero pins, a quarantined repo is skipped across both phases, a path-scoped-withheld blob never reaches a sink, and the cursor eventually covers every repo. Each should go red if the corresponding gate is removed.

• [P3] Smaller items
  crates/gitlawb-node/src/main.rs:504
  The sweep is spawned unconditionally (unlike auto-sync at main.rs:492, gated on if config.auto_sync), so it full-scans up to 100 repos hourly and runs the missing-set DB queries even when neither IPFS nor Pinata is configured — gate the spawn on a configured backend. There is no deadline on either spawn_blocking; a stalled git child leaks the blocking thread and delays shutdown, which only checks the signal between repos. And three DB calls use ? (reconciliation.rs:205, :225, :322), aborting the entire pass on a transient error, where every sibling check continues and skips just the one repo — make them consistent.

Net: the confidentiality core is solid and I verified it does not leak; the blocker is the Pinata NOT NULL regression, and the durability guarantee has real coverage holes (large-repo tails, idle repos) plus the stale base. All fixable without touching the visibility design.

Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/gitlawb-node/src/reconciliation.rs`:
- Around line 90-101: Replace the numeric offset cursor logic in the repository
sweep around list_all_repos_deduped with stable ordering and keyset pagination:
order repositories by an immutable deterministic key, filter after the
previously scanned repository id, and persist the last scanned id as the cursor.
Update the cursor type and reset behavior for empty or completed sweeps while
preserving the REPOS_PER_PASS limit and avoiding skipped repositories when
updated_at changes.
- Around line 257-267: Update the reconciliation flow to capture the lengths of
ipfs_candidates and pinata_candidates before they are moved into pin calls, then
record their sum as gaps found. Keep gaps found recording independent of the
repo_filled > 0 guard so failed pins still count detected gaps, while continue
recording gaps filled from pinned_ipfs and pinned_pinata.

---

Nitpick comments:
In `@crates/gitlawb-node/src/reconciliation.rs`:
- Line 205: The per-repository DB calls currently propagate errors and abort
run_pass, unlike the surrounding resilient loop. In the repository-processing
flow, replace the ? handling for filter_ipfs_pinned_oids,
filter_pinata_pinned_oids, and list_all_encrypted_blobs with match-based
handling that logs a warning and continues to the next repository on error,
while preserving successful results.

Traced the new reconciliation module against the base-branch code it calls into (push_delta.rs, visibility_pack.rs, smart_http.rs) rather than reviewing the diff in isolation. The durability idea and the quarantine/visibility reuse are sound; one finding should block merge.

Findings

• [P1] Make REPO_SCAN_DEADLINE actually kill the git subprocess it wraps
  crates/gitlawb-node/src/reconciliation.rs:530
tokio::time::timeout racing a spawn_blocking handle only stops awaiting it on elapse, it doesn't abort the blocking task. Inside that closure, list_all_objects and blob_paths (via replicable_blob_set) shell out to git cat-file/git rev-list/git ls-tree with plain Command::output(), no process_group, no timeout of their own — blob_paths runs git ls-tree once per reachable commit. On a slow or pathological repo, "deadline exceeded, skip" fires while the blocking thread and however many git children were mid-walk keep running unbounded, and the cursor revisits the same repo every pass. smart_http.rs already has the fix for this exact class (process_group(0) + a kill-on-drop guard that reaps the whole process group, built for the #174 watchdog gap) — reuse it here instead of the bare timeout.

• [P2] Recheck visibility rules, not just quarantine, before pinning
  crates/gitlawb-node/src/reconciliation.rs:512
  Rules and is_public are fetched once per repo before the full scan and reused unchanged through both pin phases; only quarantine gets rechecked immediately before pinning. If an owner narrows visibility mid-scan, the sweep pins/reseals against the stale, more-permissive snapshot. For content-addressed public pins that's effectively irreversible. Recheck visibility the same way quarantine is already rechecked, right before each pin phase.

• [P3] Fix the vacuous spawn-gate test
  crates/gitlawb-node/src/reconciliation.rs:790
test_spawn_gate_is_not_broken_by_constant_typos asserts SWEEP_INTERVAL_SECS != 0 and never touches config or calls spawn(). It would pass unchanged if the actual empty-config short-circuit were deleted or inverted. Either delete it or test the real gate against a minimal Config.

Actionable comments posted: 2

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/gitlawb-node/src/git/mod.rs`:
- Around line 117-129: Update GitCommand::output() to configure both stdout and
stderr as Stdio::piped() before calling spawn_registered(), so
wait_with_output() captures command output. Leave GitCommand::spawn() unchanged
for callers that manage stdio themselves.

In `@crates/gitlawb-node/src/git/push_delta.rs`:
- Around line 179-187: Update GitCommand::output in the git command
implementation to explicitly configure stdout and stderr as piped before
invoking the underlying command output operation. Preserve the existing output
and error propagation behavior so list_all_objects and
list_all_objects_with_type receive the subprocess streams without requiring
call-site changes.

---

Outside diff comments:
In `@crates/gitlawb-node/src/git/visibility_pack.rs`:
- Around line 24-34: Update GitCommand::output() in the git module to force
command stdout to be piped before executing, while preserving existing stderr
and status handling. This ensures callers such as assert_all_refs_are_commits
receive refname output when no explicit stdout configuration is provided.
- Around line 160-181: Update GitCommand::output() in git/mod.rs to capture and
return the child process stdout and stderr when no explicit stdio configuration
is provided. Preserve the existing command execution and status handling so
callers such as the rev-list and ls-tree flows in visibility_pack.rs receive
their output for blob-path and visibility processing.

I found issues that need to be addressed before this is ready.

Findings

• [P1] Recompute the object exposure set after a visibility change
  crates/gitlawb-node/src/reconciliation.rs:172
  The blocking scan derives object_list from the rules captured at the start of the pass, but the pre-upload recheck at lines 247-276 only asks whether / remains anonymously listable. If an owner adds a path rule such as /secret/** while the scan is running, root access still passes and the old list still contains the newly-withheld blob, so lines 338-349 publish it to IPFS/Pinata in plaintext. Re-derive the replicable set from the fresh rules and repo state (or otherwise synchronize the permission decision with the upload) before any irreversible public write; Phase 2 should likewise use the fresh identity/state when deriving recipients.

• [P1] Keep the pin listing compatible with Pinata-only rows
  crates/gitlawb-node/src/db/mod.rs:2321
  This change deliberately permits and inserts cid = NULL for a Pinata-only pin, but PinnedCidRecord.cid remains a String and this query decodes it as one. The first successful Pinata-only upload therefore makes list_pinned_cids fail with SQLx's unexpected-NULL error; /api/v1/ipfs/pins maps that error to a 500, which also breaks the CLI consumers of that endpoint. Make the response field nullable or explicitly filter/represent non-local rows, and add coverage for the supported Pinata-only configuration.

• [P1] Make timeout cancellation atomic with process registration
  crates/gitlawb-node/src/git/mod.rs:179
  A timeout can set canceled and drain the registry after the post-spawn load at line 180 but before line 208 inserts the new process group. That group then misses the only kill sweep and the detached spawn_blocking task continues in wait_with_output() past REPO_SCAN_DEADLINE. Coordinate the cancellation check and registration with the timeout's sweep (and kill the entire -pgid in the immediate-cancel branch, rather than only the child PID) so no child can be registered after cancellation has already won.

• [P2] Bound the encrypted recovery phase too
  crates/gitlawb-node/src/reconciliation.rs:413
withheld_blob_recipients performs a full history walk and one git ls-tree per reachable commit, then the result is encrypted and uploaded without a deadline or work cap. Unlike the preceding scan it has neither REPO_SCAN_DEADLINE nor a ScanContext, so a large or stalled path-scoped repository can hold the sweep and a blocking worker indefinitely, leave its Git children outside the timeout cleanup, and then trigger an unbounded recovery upload. Run this phase under the same cancellation/process tracking and a restartable per-pass budget.

• [P2] Apply the repository cursor and limit in SQL
  crates/gitlawb-node/src/db/mod.rs:1262
list_all_repos_deduped_stable does a fetch_all of every deduped repository; run_pass only finds the cursor and slices 100 after that allocation. Consequently the advertised 100-repository cap does not bound the hourly query, transfer, dedup work, or memory use, and deleting the cursor row resets the scan to the first page. Make this a real keyset query (id > cursor, ordered by id, with LIMIT) and explicitly wrap only when the bounded query is exhausted.

• [P2] Do not count disabled backends as reconciliation gaps
  crates/gitlawb-node/src/reconciliation.rs:278
  The worker intentionally starts when either backend is configured, but it always computes and counts both missing sets. On a valid Pinata-only node, every object is added to ipfs_missing and gaps_found even though ipfs_pin::pin_new_objects immediately no-ops for an empty IPFS URL; the converse happens for an IPFS-only node. That makes the new counters permanently report unfillable gaps and can drive false durability alerts. Only compute and count a backend's missing set when that backend is enabled.

I found issues that need to be addressed before this is ready.

Findings

• [P1] Preserve structural objects when refreshing visibility
  crates/gitlawb-node/src/reconciliation.rs:279
  The initial scan correctly uses replicable_objects_fail_closed, which preserves commits and trees while applying the allow set only to blobs. The subsequent refresh instead intersects every OID with replicable_blob_set, whose contract explicitly contains blobs only. Consequently a missed push-time pin for a commit or tree is never repaired by either backend, and the resulting off-node object set cannot reconstruct the repository. Reapply the type-aware fail-closed filter with the fresh blob set (or otherwise retain non-blobs).

• [P2] Keep the IPFS-pins response compatible with Pinata-only rows
  crates/gitlawb-node/src/db/mod.rs:159
  New Pinata-only records intentionally have cid = NULL, but /api/v1/ipfs/pins serializes those records unchanged while gl ipfs list reads only cid. A successful Pinata-only pin therefore renders as ?, despite the response containing a usable pinata_cid; this changes the documented local-pin response contract and breaks its CLI consumer. Return a usable backend-aware CID or update the endpoint and consumer together.

• [P2] Do not run the refreshed visibility walk on a Tokio worker
  crates/gitlawb-node/src/reconciliation.rs:273
replicable_blob_set performs synchronous Git history traversal (rev-list and an ls-tree per reachable commit), yet this second invocation is made directly from run_pass, outside both spawn_blocking and REPO_SCAN_DEADLINE. A large or stalled repository can therefore block a Tokio worker indefinitely after the initial bounded scan and delay shutdown or unrelated async work. Fold this recomputation into the bounded scan, or give it equivalent cancellation-aware blocking execution.

• [P2] Register every Git subprocess in the timed scan
  crates/gitlawb-node/src/git/store.rs:69
  The new timeout only terminates process groups registered through GitCommand, but blob_paths calls this raw Command::new("git") via head_commit during both reconciliation scans. If that rev-parse stalls, the timeout stops awaiting the blocking task without being able to signal or reap its child, leaving a blocking worker behind despite the advertised per-repo deadline. Route scan-path subprocesses through the registered wrapper (and audit the helpers reached by the scan).

• [P2] Bound the pin phase as well as the Git scan
  crates/gitlawb-node/src/reconciliation.rs:351
  Each backend is allowed to process 50,000 missing objects serially, and the new deadline covers only the earlier Git walk. With an unavailable backend, this loop awaits one upload at a time until the client timeout for every object, so a single repository can hold the sole sweep task for days and prevent the cursor from reaching other repositories. Apply a per-repository wall-clock budget/cancellation to pinning (with bounded batching or concurrency).

Confirmed jatmn's structural-objects P1 by execution rather than restating it: on a public repo with no rules, the fail-closed scan yields 4 structural objects and the intersect at reconciliation.rs:279-282 yields 0. It is unconditional, not narrowing-only. The rest below is what this head still needs, and the first item is a scope call I am settling as lead.

Findings

• [P1] Split this into three PRs before the next round
  crates/gitlawb-node/src/reconciliation.rs:1
  Three of the five findings on this head were introduced by the fixes for the previous round's findings, and the diff has grown from 607 to 1032 lines, mostly in a subprocess-registry layer bolted onto shared serving-path helpers. That is a loop that costs more each turn. Land (1) the pinned_cids nullable-cid semantics plus migration 12 and the gl ipfs list consumer, (2) the GitCommand process-group registry on its own with tests on the serving path it now changes, and (3) the sweep on top. The durability need is real and I want it in; the current shape is not reviewable one round at a time.

• [P1] Delete or rewrite the spawn-gate test, it passes with the gate removed
  crates/gitlawb-node/src/reconciliation.rs:573
  I removed the if config.ipfs_api.is_empty() && config.pinata_jwt.is_empty() { return; } block at :41-44 and re-ran the module: both tests still pass. tokio::spawn only enqueues the task, and the test has no await after the call, so it is never polled. The doc comment at :566-572 asserts the opposite. Extract should_spawn(&Config) -> bool and assert both directions.

• [P2] Match the loop's own convention at the fresh-visibility recompute
  crates/gitlawb-node/src/reconciliation.rs:278
  This is the only ? inside for repo in &batch; every sibling failure warns and continues. The cursor is advanced past the whole batch at :118 before the loop starts, so one repo's git error abandons up to 99 already-selected repos, and they wait for a full cursor wrap before anything looks at them again.

• [P2] Do not hold the scan registry lock across the child wait
  crates/gitlawb-node/src/git/mod.rs:194
  The cancel-after-spawn branch takes ctx.registry.lock() and then calls child.wait_with_output() under it, while the deadline handler at reconciliation.rs:202 acquires that same std::sync::Mutex from async context. A process group that ignores SIGTERM blocks a tokio worker on the lock. Snapshot the pgids under a short lock and kill outside it, and use unwrap_or_else(|e| e.into_inner()) at both sites so a poisoned lock cannot end the sweep task permanently.

• [P2] Ship the v12 upgrade-path test with the migration
  crates/gitlawb-node/src/db/mod.rs:889
  A fresh-DB suite runs the migration array from scratch and cannot see an upgrade-path bug; migration_v11_creates_owner_did_column at db/mod.rs:3665 is the pattern to mirror. Seed the legacy cid = pinata_cid row shape the migration comment says has_ipfs_cid handles, and assert the classification. I could not determine whether a Kubo add and a Pinata upload return the same CID for the same bytes; if they ever do, cid IS DISTINCT FROM pinata_cid marks a genuinely pinned object as a permanent gap and re-uploads it every pass. That test should settle it either way.

• [P3] Give the sweep an operator switch and document it
  crates/gitlawb-node/src/main.rs:502
  Any node with IPFS or Pinata configured now runs hourly full-object scans over up to 100 repos, with no way to turn it off and no mention in the operator docs. Auto-sync is the precedent: config.auto_sync, README.md:344, .env.example:152.

• [P3] Anchor the pass delta, not the merged manifest
  crates/gitlawb-node/src/reconciliation.rs:523
  The push path anchors only what it sealed (api/repos.rs:1175); the sweep merges list_all_encrypted_blobs into every anchor, so each pass republishes entries already on the ledger. Not a new disclosure, since past deltas cover the same OIDs, but it is a paid permanent write and it diverges from the established pattern.

Rechecked head beae7cd against my prior review on 88e49b5 and re-verified each finding against the checkout (not just blind-search candidates). The latest round fixes a lot of the earlier durability and API-contract work (structural-object refresh, keyset repo pagination, nullable cid migration + test, Pinata-only /api/v1/ipfs/pins synthesis, spawn-gate tests, GITLAWB_RECONCILIATION_SWEEP, bounded git scans, and pin-phase timeouts). The confidentiality core still looks careful. I still see PR-owned issues that need to be addressed before this is ready.

Findings

• [P1] Re-validate quarantine and visibility immediately before each irreversible public pin
  crates/gitlawb-node/src/reconciliation.rs:418
  Phase 1 re-fetches quarantine, is_public, and rules, re-runs the fail-closed refilter, and only then builds the missing sets. Neither the up-to-300s refilter (~302–351) nor the subsequent pin phases (~418–449, up to 600s total) re-check quarantine or visibility. If the owner quarantines the repo or narrows visibility during either window, the sweep can still publish content to IPFS/Pinata — and the code itself notes that stale public pins are effectively irreversible (~248). Add the same pre-upload gate used at ~250–290 immediately before each backend pin (or inside the pin loops), not only before the git scan.

• [P2] Phase 2 still uses stale repo identity for encrypted recovery
  crates/gitlawb-node/src/reconciliation.rs:512
  Phase 2 re-fetches fresh_repo and passes fresh_repo.is_public to listable_at_root, but withheld_blob_recipients is called with batch-snapshot repo.is_public and repo.owner_did. Phase 1 already uses fresh_repo for the refilter (~297–298). If ownership or is_public changes mid-pass, recovery copies can be sealed for the wrong owner/recipient set and the Arweave manifest can carry a stale owner_did (~592). Pass fresh_repo fields into the phase-2 blocking call the same way phase 1 does.

• [P2] Legacy record_pinata_cid updates can falsely mark objects as locally IPFS-pinned
  crates/gitlawb-node/src/db/mod.rs:2410
  Migration v12 and has_ipfs_cid correctly treat legacy rows where cid = pinata_cid as Pinata-only, but record_pinata_cid's ON CONFLICT path updates only pinata_cid and leaves the old cid untouched. When Pinata returns a new CID for such a row, has_ipfs_cid / filter_ipfs_pinned_oids see cid IS NOT NULL AND cid IS DISTINCT FROM pinata_cid and classify the object as locally IPFS-complete even though cid is still the old Pinata fallback. Both the push path (ipfs_pin::pin_new_objects) and the sweep then skip local IPFS repair permanently. Clear or NULL cid when updating pinata_cid on legacy equal-cid rows (or when the stored cid equals the previous pinata_cid), and add a test that re-pins a legacy row with a different Pinata CID.

• [P2] record_pinned_cid cannot repair a stale wrong local CID
  crates/gitlawb-node/src/db/mod.rs:2240
  The new v12 ON CONFLICT upsert only updates cid when cid IS NULL OR cid = pinata_cid. If a row already has a wrong local cid that differs from pinata_cid, a later successful IPFS pin is ignored, has_ipfs_cid / filter_ipfs_pinned_oids treat the object as complete, and both the sweep and push path skip repair permanently. Allow overwrite when the stored CID is known-bad or add an explicit repair path for reconciliation.

• [P2] Pinata-only nodes still inflate IPFS gap metrics
  crates/gitlawb-node/src/reconciliation.rs:354
  This was in my prior review and is still open on this head. _ipfs_enabled is computed but unused; ipfs_missing and gaps_ipfs are always built and counted even when config.ipfs_api is empty, while pin_new_objects("", …) no-ops. Pinata-only deployments permanently report unfillable IPFS gaps in gitlawb_reconciliation_gaps_found_total. Gate IPFS missing-set computation, gap counting, and the IPFS pin call behind !config.ipfs_api.is_empty() the same way Pinata is gated at ~383.

• [P2] Bound the encrypted recovery upload phase
  crates/gitlawb-node/src/reconciliation.rs:571
  The git walk for withheld_blob_recipients is now deadline-bounded, but encrypt_and_pin is awaited with no timeout. A repo with many withheld blobs or a slow IPFS backend can hold the sole sweep task indefinitely and delay shutdown (only checked at the top of the per-repo loop). Wrap phase 2 sealing in the same PIN_PHASE_DEADLINE (or a dedicated budget) used for public pinning.

• [P2] Do not hold the scan registry lock across child reap
  crates/gitlawb-node/src/git/mod.rs:194
  In the post-spawn cancellation branch, spawn_registered holds ctx.registry.lock() while calling child.wait_with_output(). The timeout handler in run_pass (~219) needs that same lock to snapshot pgids for SIGTERM. A git child that ignores SIGTERM blocks the async timeout path from cleaning up other registered processes in the same scan. Snapshot pgids under a short lock, release, then wait/kill outside the lock (mirror smart_http.rs's bounded SIGTERM→SIGKILL escalation).

• [P2] Add guards for the leak-class and coverage-critical sweep behavior
  crates/gitlawb-node/src/reconciliation.rs:1
  The new spawn-gate and migration v12 tests are useful, but this head still has no tests that a private repo produces zero pins, a quarantined repo is skipped across both phases, a path-scoped withheld blob never reaches a sink, or the stable cursor eventually covers every repo. metrics::tests also does not assert registration or increment behavior for gitlawb_reconciliation_gaps_found_total / gitlawb_reconciliation_gaps_filled_total. Each guard should go red if the corresponding gate is removed.

• [P3] Per-repo missing-set cap can starve the same objects every pass
  crates/gitlawb-node/src/reconciliation.rs:368
  Missing sets are built from HashSet::difference (arbitrary order), then truncate(MAX_OBJECTS_PER_REPO). Repos with more than 50k unpinned objects per backend can leave the same tail subset unselected on every hourly pass. Use deterministic ordering (OID sort) and rotate the cap window, or page within the repo.

• [P3] Filter queries still send the full uncapped object list to Postgres
  crates/gitlawb-node/src/reconciliation.rs:358
list_all_objects materializes every OID before the per-backend cap applies. filter_ipfs_pinned_oids / filter_pinata_pinned_oids then pass the entire object_list through ANY($1). Very large repos can spike memory and produce slow or failing filter queries even though pin work is capped. Batch the filter queries or cap before hitting SQL.

• [P3] Document the new operator switch
  crates/gitlawb-node/src/config.rs:89
GITLAWB_RECONCILIATION_SWEEP defaults to on and is absent from README.md and .env.example (unlike GITLAWB_AUTO_SYNC, which is documented in both). Operators cannot discover how to disable the hourly full-object scan.

• [P3] Do not log "worker started" when the sweep is gated off
  crates/gitlawb-node/src/main.rs:512
reconciliation::spawn returns immediately when neither backend is configured or reconciliation_sweep is false, but main always logs reconciliation sweep worker started. That makes runtime logs contradict the gate the new tests exercise.

• [P3] A filter DB error on one backend skips the other backend's gap-fill
  crates/gitlawb-node/src/reconciliation.rs:358
filter_ipfs_pinned_oids and filter_pinata_pinned_oids each use continue on error, aborting the whole repo iteration. A transient failure in the Pinata filter (~384–388) skips already-computed IPFS pinning; a failure in the IPFS filter (~358–362) skips Pinata work entirely. Treat filter errors per-backend (empty missing set + warn) so independent backends do not block each other.

• [P3] Mid-pass shutdown advances the cursor past unprocessed repos
  crates/gitlawb-node/src/reconciliation.rs:135
  The cursor is set to batch.last().id before the per-repo loop. A shutdown break mid-batch leaves the cursor at the batch end, so the next pass queries id > cursor and skips every unprocessed repo in the interrupted batch until the cursor wraps. Defer cursor advancement until the batch finishes, or persist per-batch progress.

• [P3] Pin-phase timeout drops the future but not in-flight uploads
  crates/gitlawb-node/src/reconciliation.rs:418
tokio::time::timeout(PIN_PHASE_DEADLINE, pin_new_objects(...)) returns an empty pinned list on expiry while per-object reqwest POSTs started inside the loop keep running (ipfs_pin.rs / pinata.rs). The timeout arms also discard partial pin progress, so gaps_found can rise while gaps_filled undercounts objects pinned before the deadline. Use a cancellation token or shared client with abort, and count partial fills before returning.

• [P3] Successful external pins count as filled even when DB persistence fails
  crates/gitlawb-node/src/ipfs_pin.rs:134
  Pre-existing in the push pin path; reconciliation now amplifies it via gaps_filled (~451–454). pin_new_objects / pinata::pin_new_objects push (sha, cid) into their return vec after a successful upload even when record_pinned_cid / record_pinata_cid fails (warn-only), so metrics overstate durable progress while the next pass retries the upload.

• [P3] Scan timeout does not fully reclaim blocking work
  crates/gitlawb-node/src/reconciliation.rs:226
  When REPO_SCAN_DEADLINE fires, the async side SIGTERMs registered pgids once and moves on without a grace period, SIGKILL escalation, or reap. tokio::time::timeout also does not cancel the spawn_blocking task, so timed-out scans can keep running in the pool. On non-Unix targets the kill path and process_group(0) registration are compiled out (git/mod.rs:181–185, reconciliation.rs:217–231), leaving orphan git children with no termination hook.

• [P3] Quarantine recheck is deferred until after the full git scan
  crates/gitlawb-node/src/reconciliation.rs:166
is_repo_quarantined is not checked until after the scan completes (~250). A repo quarantined during the up-to-300s walk still pays the full git I/O cost every pass before being skipped. This is wasted work, not a pin leak (quarantine is rechecked before pinning), but it matters on pathological or repeatedly quarantined repos.

• [P3] Pin reads still bypass GitCommand cancellation wiring
  crates/gitlawb-node/src/git/store.rs:294
  This PR routes scan/refilter git through GitCommand, but ipfs_pin::pin_new_objects, pinata::pin_new_objects, and encrypt_and_pin still read bytes via store::read_object, which uses plain Command::new("git") (pre-existing). Pin-phase timeouts therefore cannot terminate stalled cat-file children the way scan timeouts can. Finish routing read paths through the registered wrapper or an equivalent cancellation hook.

• [P3] gaps_found double-counts objects missing on both backends
  crates/gitlawb-node/src/reconciliation.rs:410
repo_gaps = gaps_ipfs + gaps_pinata adds the per-backend missing-set sizes. One OID absent from both backends increments gitlawb_reconciliation_gaps_found_total twice even though the metric description says "objects that should be pinned but are not." Count unique OIDs or record per-backend metrics separately.

• [P3] Mid-pass shutdown overreports repos scanned
  crates/gitlawb-node/src/reconciliation.rs:615
  A shutdown break can exit the per-repo loop early, but run_pass still returns (batch.len(), …). The pass-complete log therefore reports the full batch size even when only a prefix was processed.

• [P3] PR widens exposure on the already-unsigned pins route
  crates/gitlawb-node/src/api/ipfs.rs:238
/api/v1/ipfs/pins was already on the unsigned ipfs_routes merge before this PR (server.rs:220, tracked in #121). This change adds pinata_cid to every entry, so anonymous callers can now enumerate node-wide Pinata CIDs without signing. If the index is meant to stay authenticated (#134 on the CLI side), omit backend-specific fields for anonymous reads or gate the route.

• [P3] list_pins can emit "cid": null
crates/gitlawb-node/src/api/ipfs.rs:236
  Migration v12 allows cid to be NULL, and display_cid is p.cid.or_else(|| p.pinata_cid). A row with both columns NULL serializes "cid": null, breaking the prior always-string contract. Filter incomplete rows or guarantee both backends write at least one CID before listing.

• [P3] Durability-backstop wording overstates behavior when sweep is gated off
  crates/gitlawb-node/src/git/push_delta.rs:265
  Push-time pin failures log that the reconciliation sweep backstops them, and main describes the sweep as filling gaps so dropped replication never means data loss. should_spawn is a no-op when neither IPFS nor Pinata is configured or when reconciliation_sweep=false, so those nodes have no backstop. Tighten the comments/logs to match the gate, or document the dependency on a configured backend.

Re-reviewed head beae7cd by execution rather than by reading the diff. The confidentiality core on a canonical repo still holds up: I could not construct a rule shape, is_public value, or DID form that gets a withheld blob into a sink through the normal path. Two things changed my read this round, and both came from looking at merged behavior instead of the diff.

Findings

• [P1] Skip mirror rows in the sweep, or resolve them to a canonical row first
  crates/gitlawb-node/src/reconciliation.rs:166
  Mirror rows are written by upsert_mirror_repo with is_public = true hardcoded (db/mod.rs:1032), and nothing replicates visibility rules to a mirror: sync.rs has zero references to rules. The sweep loads rules with list_visibility_rules(&repo.id), so for a mirror with no canonical twin it gets an empty rule set and a public flag, and the gate here allows unconditionally. I ran the conjunction against a real DB: the mirror is returned by list_all_repos_deduped_stable, its rules are empty, and listable_at_root returns true, while the same gate still denies a private canonical repo. That makes the gate vacuous for exactly the repos whose rules this node does not have. Promisor mode usually keeps withheld blobs off disk, but a repo that was public when first mirrored is cloned Plain (sync.rs:76), and git does not delete those objects when the origin later narrows visibility. The result is an irreversible publish to IPFS and Pinata of content the origin now withholds. Pinning previously only ran on the authenticated push path against a repo whose rules this node owns, so this PR is what makes that reachable. The slash-form id test is already the established way to spot a mirror (api/repos.rs:1765, db/mod.rs:2560).

• [P1] Make sweep coverage survive a restart
  crates/gitlawb-node/src/reconciliation.rs:65
  The cursor is a local Option<String> inside the spawned task, so every process start resets the sweep to the first page. With REPOS_PER_PASS at 100 and an hourly interval, a node with more than 100 repos that restarts more often than a full cycle never reaches the tail, and idle repos are the ones with only this backstop. That is the coverage guarantee the PR is written to provide, so it needs to hold across a deploy. Note there is no node-state or key-value table in the schema today, so persisting it means new DDL, which is one more reason to land the storage change separately from the worker.

• [P1] Split this into three PRs, as asked last round
  crates/gitlawb-node/src/reconciliation.rs:1
  This is the second time, so I am settling it rather than restating it. The diff has gone 607 to 1032 to 1311 lines across the rounds where I asked for the split. Findings continue to trace to previous rounds' fixes rather than to the original defect: the fresh_repo re-fetch added for a prior finding is used for the phase 1 gate but not for the phase 2 seal two lines later, the nullable-cid work introduced the classification state machine below, and the process-group registry introduced the lock-across-wait problem jatmn has now filed twice. A wrong answer here publishes content permanently, which is the wrong risk profile for a change this shape. Land (1) the pinned_cids nullable-cid semantics with migration v12 and the /api/v1/ipfs/pins consumer, (2) the GitCommand process-group registry with tests on the serving path it changes, then (3) the sweep on top. Each is reviewable in one round; this is not.

• [P2] Test the behavior this PR exists to change
  crates/gitlawb-node/src/reconciliation.rs:418
  I emptied both missing sets right before the pin phases, so the sweep detects gaps and repairs nothing, and ran the full suite: 517 passed, 0 failed. Gap repair is the entire premise and nothing holds it. The same is true of the pieces underneath it. Replacing record_pinned_cid's conditional upsert with DO NOTHING, which removes the only path by which a Pinata-only row ever becomes IPFS-pinned, leaves all 63 db tests green, and reverting record_pinata_cid's NULL bind to the legacy cid = pinata_cid fallback also leaves them green, including the new v12 test. The v12 test is genuinely load-bearing for the DDL and the classification predicate, so this is about the writers, not that test.

• [P2] Stop inferring IPFS provenance from CID inequality
  crates/gitlawb-node/src/db/mod.rs:2348
has_ipfs_cid and filter_ipfs_pinned_oids decide "locally pinned" with cid IS NOT NULL AND cid IS DISTINCT FROM pinata_cid, which treats a value comparison as a provenance record. A CID is a function of the bytes, so this is correct only while the two backends happen to disagree. Today they likely do, since Kubo is called with cid-version=1&raw-leaves=true (ipfs_pin.rs:30) and the Pinata v3 upload sends plain multipart with no codec parameters, but that is a third party's chunking default, not an invariant this repo controls or tests. If they ever agree, a successful Pinata write downgrades a correctly pinned row to not-pinned and the object is re-read, re-uploaded and re-counted as a gap every hour. This is the question I raised last round and it is still open; the fix is to record provenance rather than infer it, for example backfilling legacy equal rows to NULL in the migration and reducing the predicate to cid IS NOT NULL. Worth compiling before you commit to the exact shape.

• [P2] Delete or rewrite the spawn-gate test, it still passes with the gate removed
  crates/gitlawb-node/src/reconciliation.rs:679
  Re-ran my check from last round on this head: I replaced the early return at :56-61 with let _ = should_spawn(&config); and all 6 reconciliation tests stayed green, including test_spawn_gate_skips_when_no_pin_backends_configured. The four should_spawn cases you added are real and do test the predicate, so keep those. It is the test that calls spawn() and asserts nothing that should go, or return something from spawn() it can assert on.

jatmn's round on this head is otherwise still open as written, and I am not going to re-litigate it here. I confirmed one of theirs directly: _ipfs_enabled at reconciliation.rs:354 is declared and never read, while pinata_enabled does gate at :383, so a Pinata-only node counts every object as an unfillable IPFS gap forever.

One scoping note on their phase 2 finding, so the fix stays a one-liner. Passing fresh_repo.owner_did and fresh_repo.is_public at :512-514 is right and worth doing, but the only columns any code updates on repos are updated_at and quarantined (db/mod.rs:1327, :1469). There is no public/private toggle and no ownership transfer, so the stale values are identical to the fresh ones today and this is about not leaving the trap armed. The mid-pass narrowing that actually can happen comes through the visibility rules table and quarantine, so that is where a recheck earns its keep.

Net: the visibility design on canonical repos is still the strong part of this work and I want the durability backstop in. The mirror path is a genuine gap that only appears in merged behavior, the coverage guarantee does not survive a restart, and the premise has no test. Those are three different subsystems, which is the argument for the split rather than a seventh round on one branch.

Re-reviewed at 9588098b by execution. Two commits since 07a48788. Reconciliation suite 14/14 green locally; sweep_never_pins_withheld_blob_in_cleartext passes. The sweep core from prior rounds still holds: fresh authz_deadline at the mid-scan call site, mirror rows skipped, and the three main-restoration items from the rebase round are still present.

Findings

• [P1] Keep the lockfile MSRV-compatible on Rust 1.91
  Cargo.lock:1

  9588098b advances aws-credential-types from 1.2.14 (main) to 1.3.0, which requires rustc 1.94.1. MSRV and Docker CI both fail on that error. Narrow the lockfile change to the crates that actually need bumping without pulling the AWS smithy stack past MSRV, or move the dependency refresh out of this PR.

• [P2] Finish dropping the stale-CID sweep repair claim
  crates/gitlawb-node/src/db/mod.rs:2641

  You took the right option from the last round (drop the sweep claim, keep push-path upsert repair). The migration test comment at :4719 reflects that. The production docstring on record_pinned_cid still says "A stale local CID must be repairable by the sweep." That contradicts filter_ipfs_pinned_oids (cid IS NOT NULL at :2818) and the gap logic in ipfs_pin.rs. Align the docstring with the test comment.

• [P3] Add a run_pass test that a spent scan_deadline cannot starve the mid-scan refilter
  crates/gitlawb-node/src/reconciliation.rs:522

  The wiring is correct: authz_deadline is allocated fresh at :522 and passed to refilter_public_objects at :529. refilter_starves_on_spent_deadline_but_runs_on_fresh_deadline pins the helper's contract only. Reverting the call site to scan_deadline would not flip any test. The jitter added in 4c603312 is fine but does not close this gap. Either plumb the deadline constant through run_pass's signature for a call-site regression test, or add an integration test that forces a spent scan budget and asserts pin work still runs.

One process note, not a finding: jatmn's 08-15 review still carries CHANGES_REQUESTED with guidance to split the branch into pin-state / policy-fence / sweep PRs. That split ask is separate from the three items above.

Not an ask, recorded only: the open CodeRabbit thread on ScanContext.canceled / escalate_kill is stale on this head (rg finds no matches); subprocess lifecycle here uses visibility_pack::run_bounded_git inside spawn_blocking closures.

Re-reviewed at 0832c23d by execution. The MSRV lockfile regression and the record_pinned_cid docstring from my 9588098b round are closed on this head: rustup run 1.91 cargo check -p gitlawb-node finished clean, CI is 12/12, and reconciliation is 14/14 green including sweep_never_pins_withheld_blob_in_cleartext. A cross-model refute pass on this head surfaced two trust-boundary gaps I verified in the tree.

Findings

• [P2] Filter structural git objects under path-scoped denies before public pin
  crates/gitlawb-node/src/reconciliation.rs:463

  replicable_objects_fail_closed only withholds blobs. Reachable tree and commit OIDs pass through unchanged, so a public repo with /secret/** denied still pins the HEAD:secret tree to IPFS. That tree names secret.txt and carries the withheld blob OID even when the blob itself is correctly excluded. I reproduced the filter behavior locally (secret_tree passes while secret_blob does not). Extend sweep_never_pins_withheld_blob_in_cleartext to assert the secret tree OID never lands in pinned_cids and the mock IPFS endpoint never receives it.

• [P2] Bump policy_epoch in the same transaction as visibility-rule writes
  crates/gitlawb-node/src/db/mod.rs:3468

  set_visibility_rule commits the INSERT/UPDATE and only then calls bump_repo_policy_epoch in a second statement. PolicyFence is epoch-only, so a narrowing can be visible to list_visibility_rules while is_current() still reads true until the bump lands. pin_new_objects_stops_mid_batch_when_policy_moves covers quarantine, not this visibility-rule gap. Wrap the rule write and epoch bump in one transaction, or add a regression that applies a narrowing between the two statements and proves the pin loop aborts before the next upload.

One process note, not a finding: expect rebase friction with #173, #324, and #330 after those land.

Not an ask, recorded only: the open CodeRabbit thread on ScanContext.canceled / escalate_kill is stale on this head (rg finds no matches); resolving it with this review.

I found issues that need to be addressed before this is ready.

Overall guidance: why this review keeps producing findings

I do not want another cycle where one cited line is fixed, a nearby variant remains, and the next review reports it as a new issue. The findings below are not seven unrelated mistakes. They are consequences of a few contracts that are spread across too many layers and have not yet been defined or tested end to end.

1. The branch contains a dependency stack, not one isolated sweep

The worker in reconciliation.rs depends on several contracts that this same branch is also inventing or changing:

When foundations and their consumer land together, a local fix in one file changes the assumptions of several other files. That is the main source of review drip. The sweep should be the consumer of already-established pin-state, visibility, and fencing contracts; it should not be the place where all of them are designed simultaneously.

2. The security property is located at the effect, but checks are being added at earlier stages

The irreversible property is: an object must not begin public publication under authorization that is no longer current. The current flow crosses multiple asynchronous boundaries:

repo page snapshot
  → initial rules/quarantine read
  → bounded Git object scan
  → visibility re-derivation
  → backend missing-set query
  → fence capture/check
  → object read / recipient resolution / encryption
  → HTTP upload
  → DB persistence

Several earlier rounds added a guard at one point in that sequence without specifying which policy version owns the final effect. That produces structurally similar follow-ups: a fresh deadline at one re-filter but not another, a fence captured around one rules read but not bound to dispatch, or a blob filter that does not cover trees. The atomic-epoch and upload-boundary findings below are two manifestations of the same missing transaction model.

Before changing code again, write down the invariant for every transition:

Then implement that table once in shared helpers used by IPFS, Pinata, and encrypted sealing. Copying similar checks into three loops has already let their ordering and timeout semantics drift.

3. Boundedness has been treated as a ceiling, but durability also requires fairness and resume semantics

The repository cursor, 50,000-object cap, Git deadlines, and pin budgets successfully bound individual passes. They do not by themselves prove eventual repair. A backstop needs both properties:

• Safety/boundedness: one repo or object cannot consume unbounded resources.
• Liveness/fairness: a repeatedly failing early repo/object cannot permanently prevent later missing work from being attempted.

The current design persists progress between repositories but not within a repository/backend. The starvation finding is therefore not another timeout nit; it is the missing liveness half of the cursor design. Define continuation, rotation, retry/backoff, and terminal/reset behavior together. Tests should run at least two passes and demonstrate that the second pass reaches work the first pass could not.

4. Phase eligibility and backend capability need an explicit matrix

The worker currently uses “announceable to anonymous” as an early repo-wide gate, although public pinning and encrypted recovery answer different policy questions. It also uses “IPFS or Pinata configured” as the worker spawn gate, although only public reconciliation supports either backend and encrypted recovery requires local IPFS.

Decide and document this matrix before wiring more conditions:

Use separate predicates for repository eligibility and phase/backend capability. That makes startup logs, docs, metrics, and tests derive from the same model instead of each embedding a slightly different interpretation.

5. Tests need to bind each layer, not merely the final property

The suite has improved substantially, but several tests remain green when one of multiple redundant guards is removed. For example, a test that proves a withheld blob is absent after the whole pass does not prove the pin-boundary recheck works if an earlier filter already removed the blob. That is why a property can appear covered and still produce another timing finding.

For every security or durability boundary, add a test that fails when that specific layer is removed:

Use mutation hooks, injected deadlines, fake readers, and request counters where needed. The test should go red when the exact gate under review is deleted, not only when every defense is removed.

6. Recommended sequence for one convergent author pass

Please address this by root cause and push one coherent result, rather than replying to findings one at a time:

1. Freeze scope now. Move strict-Ed25519 and audit remediation out of the sweep stack. Do not add more migrations, API reshaping, or unrelated cleanup while the invariants below are being repaired.
2. Resolve ownership/overlap first. Decide merge order with #172/#173 for tree visibility and #368 for h2. Reuse those fixes rather than implementing competing versions here.
3. Make the two product decisions. Decide whether private encrypted recovery is required for #218 and whether Pinata is ever an encrypted sink. Update the PR body before code so reviewers assess one explicit contract.
4. Define the contracts in writing. Pin-state response schema, policy/epoch transaction, dispatch-fence semantics, object scheduling/resume, and the phase-capability matrix should each have one authoritative description.
5. Add the seam-specific failing tests. Create the regression shapes in the table above before changing implementation. Confirm each test fails for the intended missing layer.
6. Fix shared foundations before the worker. Land/implement atomic policy mutation, shared path-aware object filtering, backend provenance, and dispatch coordination. Avoid three near-copy loop fixes where one helper can own the invariant.
7. Fix sweep scheduling and phase separation. Add fair per-object continuation/rotation, then split public and encrypted phase eligibility according to the decided matrix.
8. Run the complete horizontal audit once. For IPFS, Pinata, and encrypted recovery, trace candidate derivation → current authorization → preparation → irreversible effect → persistence → retry/resume. Check normal, timeout, DB failure, policy narrow, quarantine, shutdown, and restart behavior.
9. Rebase and validate once at the end. Re-run MSRV, locked build, clippy/fmt, full workspace tests with PostgreSQL, raw cargo audit without new suppressions, and both-backend integration tests. Refresh the PR body and request one full re-review only after that set is green.

The goal is not to add more defensive checks. It is to establish a small number of contracts that make the unsafe or non-progressing states unrepresentable, then test those contracts at the exact seams where effects occur. If that is done as one pass, the next review should be verification rather than another discovery round.

Needs maintainer decision

• Resolve the requested split/freeze before another implementation round
  crates/gitlawb-node/src/reconciliation.rs:1
  The scope guidance from my b44c951 review remains unresolved. Please follow the freeze/split and ordered convergence plan above, or get explicit maintainer agreement to keep the stack together before making another implementation push.

• Decide whether #218 intentionally excludes private-repository recovery
  crates/gitlawb-node/src/reconciliation.rs:416
  The implementation and PR body explicitly skip private/non-announceable repos, so I am not treating this as an accidental code defect. It does leave a scope conflict to resolve: #218 asks for the complete backstop for dropped pins and sealed recovery copies, while the existing push path creates encrypted recovery copies for private repos with path-scoped readers. A dropped post-push seal for such a repo therefore has no periodic repair path.

  Root cause: public-pin eligibility and encrypted-recovery eligibility are coupled by an anonymous root gate before the worker reaches either phase. Those are different policies: anonymous visibility should control plaintext publication, while current recipient rules should control encrypted recovery.

  Author guidance: if private recovery is in scope, restructure the pass so quarantine/repo existence gate the shared scan, then evaluate public pins and encrypted recovery independently; add an integration test for a private repo with a path-scoped reader and a missing encrypted row. If it is intentionally excluded, narrow the PR/#218 closure language and file a follow-up that records the remaining permanent recovery gap.

Findings

• [P1] Commit policy changes and their fence epoch atomically
  crates/gitlawb-node/src/db/mod.rs:3468
set_visibility_rule commits the rule upsert and only then calls bump_repo_policy_epoch through a second autocommit statement. remove_visibility_rule at :3491-3497 and set_repo_quarantine at :1545-1554 have the same shape. A sweep can therefore observe a committed restrictive rule while PolicyFence::is_current() still sees the old epoch. More seriously, if the epoch update fails after the rule statement succeeds, the API reports an error even though policy changed, and the stale epoch can persist until an unrelated later mutation happens to bump it. Every sink trusts that epoch as the sole invalidation signal, so stale plaintext objects or stale recipient sets can continue to dispatch.

  Root cause: policy_epoch is being used as a logical commit marker, but it is not part of the policy transaction it is supposed to represent. The database can expose policy state and its invalidation token from different commits.

  Author guidance: execute each rule/quarantine mutation and epoch increment on one SQL transaction/connection and commit them together. Preserve row-count semantics for a missing repo and make any failure roll back both operations. Add tests that (1) force the epoch statement to fail and prove the policy mutation rolls back, and (2) pause between the two operations under concurrency and prove no reader can observe new policy with the old epoch. Apply the same helper/transaction pattern to every policy mutation so a future call site cannot forget the bump.

• [P1] Recheck or coordinate the fence at the irreversible upload boundary
  crates/gitlawb-node/src/ipfs_pin.rs:399
  The IPFS loop checks PolicyFence at :353, then awaits has_ipfs_cid and a bounded Git read before calling pin_git_object at :470. The read is allowed to consume almost the full 120-second batch budget, so a denial or quarantine committed during that interval still lets this object's stale plaintext start uploading. Pinata has the same interval between pinata.rs:138-238. The encrypted path checks before its tag lookup, object read, key resolution, and encryption, but not before encrypted_pin.rs:219, so a reader removed during preparation can still receive a newly published envelope. Current race tests mutate while the first HTTP request is already in flight and deliberately allow that request; they do not exercise a mutation during pre-upload preparation.

  Root cause: the fence is treated as an iteration-admission check even though the security invariant is about dispatch. Long-running preparation occurs after the authorization snapshot, and no mechanism binds the final network effect to the policy version that authorized it.

  Author guidance: at minimum, re-read the fence after all DB/Git/encryption work and immediately before each POST, retaining the loop-top check only as an efficiency guard. If the intended invariant is strict—no upload may begin after a narrowing commits—close the remaining check/POST race with shared dispatch-versus-policy coordination, such as a cross-node database lease/advisory-lock protocol where an upload reserves dispatch under the captured epoch and a policy mutation cannot commit past that reservation unnoticed. Add boundary-specific tests that block the Git read, commit a narrow while it is blocked, release it, and assert the mock endpoint receives zero requests; run the same test for IPFS, Pinata, and encrypted sealing.

• [P2] Do not activate the known denied-tree leak in the new sweep
  crates/gitlawb-node/src/reconciliation.rs:463
  The underlying replicable_objects_fail_closed behavior predates this branch and is already tracked in #172: it filters blobs but admits every non-blob. The new reachable-object intersection removes dangling commits/trees, but reachability is not visibility. For a public repo with /secret/** denied, the reachable HEAD:secret tree still enters the sweep's IPFS/Pinata candidates; its bytes expose child filenames and blob OIDs even though the secret blob itself is excluded. This PR does not invent the helper defect, but it adds a periodic caller that republishes its output while claiming fail-closed visibility filtering.

  Root cause: the object filter equates “structural” with “safe” and applies path policy only to blobs. Git trees are themselves path-bearing content, and a global reachable set cannot tell whether a tree is reachable only through a denied path.

  Author guidance: land the #172 allowed-tree derivation before enabling the sweep, or share the same path-aware object-policy helper here so push, CID serving, and reconciliation cannot drift. Derive allowed blobs and allowed trees from one bounded walk, then intersect candidates by object type; keep commits/tags only under the explicitly documented metadata policy. Extend sweep_never_pins_withheld_blob_in_cleartext with the denied directory's tree OID and assert both that no pin row is written and that neither mock backend receives its bytes. Also test a deduplicated tree reachable at both an allowed and denied path so “allowed wins” behavior is deliberate.

• [P2] Rotate or persist progress within a capped object set
  crates/gitlawb-node/src/reconciliation.rs:289
  Each pass sorts missing OIDs and truncates to the same first 50,000. If an early missing object repeatedly consumes the bounded Git-read deadline, the pin loop reaches the expired budget and exits with the remaining prefix unattempted. That object has no successful pin row, so it sorts into the same position next hour and can consume the pass again. The persisted cursor advances between repositories only; it records no position or rotation within a repository, so later gaps—including every OID beyond the cap—can remain unattempted indefinitely. Successful early objects do drain normally; the failure is the repeatable, budget-consuming object that never leaves the prefix.

  Root cause: deterministic selection, a whole-batch deadline that one object may consume, and no object-level progress/backoff combine into head-of-line blocking. The cap bounds memory/work but supplies no fairness mechanism.

  Author guidance: persist a per-repo/per-backend continuation key or rotate the starting OID after a truncated phase; alternatively give individual objects a smaller budget and record bounded retry/backoff state so one OID cannot consume the whole batch every pass. Keep the desired set deterministic, but make scheduling fair across repeated passes. Add a two-pass regression using a fake Git reader whose first-sorting OID always hangs: after pass one truncates, pass two must attempt a later OID. Add a >50,000-object scheduling-level test so the capped tail is proven eventually eligible without constructing a huge real repository.

• [P2] Preserve backend provenance in the pins API
  crates/gitlawb-node/src/api/ipfs.rs:698
  The base endpoint serializes PinnedCidRecord, which exposes cid and pinata_cid separately. This manual mapping removes pinata_cid from every response and substitutes it into cid for a Pinata-only row. Existing JSON consumers silently lose a public field, and a remote-only copy is represented as a local IPFS pin even though gl ipfs list still says it lists CIDs on the node's local daemon. That presentation also undoes the storage distinction introduced by the nullable-cid migration: the database now records provenance correctly, then the API flattens it away.

  Root cause: the response was reshaped around the legacy CLI's desire for an always-string cid, rather than defining an API contract that represents two independent durability backends. Presentation compatibility is driving persistence semantics backward.

  Author guidance: keep cid nullable and retain pinata_cid, or introduce an explicit/versioned response such as separate local_ipfs_cid and pinata_cid/backend fields. Update gl ipfs list deliberately: either list only local pins as its help promises or label the backend for every row. Add exact JSON tests for local-only, Pinata-only, both-backend, and neither-CID rows, plus a CLI test proving remote-only state is not printed as local. If removing pinata_cid is an intentional breaking API change, version and document it instead of changing the existing route silently.

• [P2] Fix h2 instead of suppressing its advisory
  .cargo/audit.toml:39
  This new ignore makes cargo audit green while reachable h2 0.4.13 remains in the lockfile. The local RustSec entry marks >=0.4.16 patched, and #368 already demonstrates the compatible update. The checked-in rationale says no fix is available, then incorrectly describes 0.4.16 as carrying the same advisory; its tracking URL is also the placeholder issues/XXX. Because the ignore is in the repository configuration, the required audit check now reports success without enforcing remediation or a real exception owner.

  Root cause: an advisory that appeared during this long-lived branch was handled as a CI unblock inside an unrelated feature PR, without verifying the advisory's patched range against Cargo's actual resolver result. That converts a transient branch-maintenance problem into durable audit policy.

  Author guidance: remove the h2 ignore and take the narrow compatible h2 update through #368 (or rebase after it lands), then rerun MSRV, locked builds, and the raw audit. Do not refresh the AWS dependency stack unnecessarily. If maintainers deliberately accept an advisory, put that decision in its own reviewed change with accurate reachability, a real tracking issue/owner, removal criteria, and a scheduled unignored check. Audit the lru comment separately as well—the locked versions already disagree with its explanation—but do not expand this sweep PR to solve unrelated dependency policy.

• [P3] Document that encrypted reconciliation requires local IPFS
  .env.example:278
  The new comment says the sweep re-derives both the public pin set and withheld recovery set when a pin backend (IPFS/Pinata) is configured. A Pinata-only configuration does start the worker and repairs public Pinata gaps, but phase 2 is guarded by ipfs_enabled at reconciliation.rs:786, and encrypt_and_pin only uploads through the local Kubo API. Operators can therefore reasonably expect encrypted recovery repair from a configuration that never performs it.

  Root cause: worker spawn eligibility describes the union of all phase capabilities, while the documentation presents that union as though every enabled phase supports either backend. Public reconciliation and encrypted reconciliation have different backend matrices.

  Author guidance: unless Pinata encrypted-envelope support is intentionally in scope, state consistently in .env.example, README, and the runbook that Pinata-only nodes reconcile public pins and that encrypted recovery reconciliation requires GITLAWB_IPFS_API. Consider expressing phase eligibility separately in code (public_backend_enabled, encrypted_backend_enabled) so startup logs and future documentation can report exactly which phases will run. Add a configuration test covering IPFS-only, Pinata-only, both, and neither, with expected public/encrypted phase capability.

Re-reviewed 07db93bd by reading the worker and tracing pin paths. CI is 12/12 green on this head. rustup run 1.91 cargo check -p gitlawb-node finished clean. The confidentiality core on canonical repos still holds: mirror rows skip, quarantine and root visibility re-check before public pin, tree-aware fail-closed filtering in the scan, policy_epoch bumps in the same transaction as visibility-rule writes, and PolicyFence::is_current runs at loop top and immediately before each HTTP POST.

0832c23d closed several prior-round asks (atomic epoch, dispatch fence before POST, pins API shape, encrypted-phase docs). Head 07db93bd only re-adds the h2 audit ignore.

Findings

• [P2] Witness denied-path trees in the sweep integration test
  crates/gitlawb-node/src/reconciliation.rs:1311

  The scan uses allowed_blob_tree_sets_bounded and tree-aware replicable_objects_fail_closed, but sweep_never_pins_withheld_blob_in_cleartext still asserts only the withheld blob OID. Extend it with the HEAD:secret tree OID: no pinned_cids row, and the mock IPFS endpoint never receives that tree's bytes.

• [P2] Add object-level fairness inside the per-repo cap
  crates/gitlawb-node/src/reconciliation.rs:289

  Missing OIDs are sorted and truncated to the same first 50,000 every pass. A head OID that repeatedly burns the Git-read or pin-phase budget can starve later gaps within the same repo, and the cursor only advances between repos. Persist a per-repo/per-backend continuation key, or rotate the starting OID after a truncated phase, and add a two-pass test where the first-sorting OID always hangs and pass two must attempt a later OID.

• [P2] Add a preparation-phase fence regression
  crates/gitlawb-node/src/ipfs_pin.rs:399

  The dispatch fence before POST is in place (476-485). Existing coverage mutates policy while the first upload is in flight and allows that in-flight object. Add a test that blocks the bounded Git read, commits a narrow during the block, releases the read, and asserts the mock endpoint receives zero requests.

One process note, not a finding: expect rebase friction with #173, #324, and #330 after those land.

Not an ask, recorded only: 07db93bd re-adds RUSTSEC-2026-0258 to .cargo/audit.toml pending #368. I am not blocking the sweep on dependency policy in this round.

I found issues that need to be addressed before this is ready.

Merge readiness

• GitHub currently reports this PR as mergeable but blocked; required-check details were unavailable to the review helper.

Findings

• [P0] Keep nested withheld objects out of public pin backends
  crates/gitlawb-node/src/git/visibility_pack.rs:542
  The new all_object_paths classifier runs git ls-tree -rz <commit> without recursive traversal. It therefore records only root entries, while the full-scan push fallback and recciliation sweep start from cat-file --batch-all-objects, which includes every reachable nested blob and tree. replicable_objects_fail_closed then treats any candidate missing from both all_blob_oids and all_tree_oids as a safe commit/tag.

  A public repository with a deny rule for /secret/** and a committed secret/nested.txt demonstrates the failure: the nested blob is absent from the classifier sets, falls through the safe-object branch, and is uploaded in cleartext to IPFS/Pinata. The same defect affects nested tree metadata. An allowed ancestor tree such as /src can still be published even when it contains a denied /src/private entry, revealing the withheld child name and object ID. Both the receive-pack full-scan fallback and the new periodic sweep use this path.

  Address the root cause by making the object/path inventory complete before applying the fail-closed filter: enumerate all reachable nested blobs and trees, preserve their actual paths, and ensure an object cannot enter a public pin sink merely because it was not classified. For trees, account for descendant visibility as well as the tree's own path, so an ancestor tree that would disclose denied entries is not published unless that exact tree is independently reachable through a fully allowed path. Add regression coverage for nested denied blobs, nested trees, and an allowed ancestor containing a denied descendant across both full-scan producers.

• [P1] Restore the full-scan shared deadline contract
  crates/gitlawb-node/src/api/repos.rs:149
  The current head fails both the stable and beta test jobs on api::repos::tests::full_scan_shares_one_deadline_across_both_phases. The failure is reproducible on both lanes, not a flake: the test reports that a costly first walk leaves a fresh budget for the second phase, and the candidate is kept instead of being fail-closed. The PR changed the full-scan classification path to the new blob/tree set construction, so it must preserve the existing single, whole-scan deadline across all children and phases. Resolve the deadline budgeting in the shared full-scan primitive, run the existing regression test on both toolchain lanes, and do not paper over the failure by weakening the assertion.

Guidance for this PR

The repeated review churn looks like a symptom of hardening individual sinks or races without first establishing and testing one complete security model for the end-to-end path. This PR crosses durability, visibility, encrypted recovery, concurrency, and persistent state; a successive series of local fixes can make each step look safe while leaving a composition edge uncovered.

Before requesting another review, please do a scoped end-to-end design and validation pass:

1. Write down the trust boundary: for each git object type, define what evidence proves it is safe to publish to each backend. Unknown, partially walked, unreachable, or unclassified objects must be denied rather than treated as structural metadata.
2. Make the set-construction primitive the single source of truth for both the push fallback and sweep. Test the primitive directly against deeply nested paths, historical objects, shared blobs, dangling objects, non-UTF8 names, and refs with non-commit targets.
3. Add sink-level authorization tests that assert the negative property, not just the filter's return value: denied objects and tree metadata must never reach IPFS, Pinata, or the encrypted sink as inappropriate, in either the post-receive or seep flow. 4. Separate security/correctness mechonism from operational hardening (cursors, budgets, metrics, jitter, and permits) in both the design and the commit structure. That lets the invariants be verified before the peripheral hardening is assessed. 5. Rerun the relevant full-scan, visibility, and backend integration tests after rebasing onto current main, and include a short matrix of the cases covered. The goal is not more tests in general, but one reviewable, end-to-end proof that the unauthorized flow cannot reach any irreversible public sink.

Re-reviewed head c2d690b4 by execution. CI is 12/12 green and MSRV check is clean. The shared-deadline fix lands: full_scan_shares_one_deadline_across_both_phases passes, and allowed_blob_tree_sets_bounded now threads one Instant through both ls-tree and cat-file phases. Nested withheld blobs still fail closed on the sweep path. The cat-file phase added in this commit does not close the tree-metadata half of the prior P0.

Findings

• [P1] Deny pathless tree OIDs before public pin backends
  crates/gitlawb-node/src/git/visibility_pack.rs:820

  Phase 2 inserts tree OIDs from cat-file --batch-all-objects with an empty path when ls-tree did not already classify them. The allow loop then calls visibility_check(..., "") on a public repo with only /secret/** deny. Empty path matches no rule, falls through to the public default, and the tree lands in allowed_trees. replicable_objects_fail_closed keeps it, and the sweep pins it to IPFS/Pinata. That tree object exposes child entry names and blob OIDs even when every blob under the denied subtree is withheld.

  I reproduced on a repo with secret/nested/deep.txt and a /secret/** deny: the nested intermediate tree OID was in allowed_trees and in the replicable set; the withheld blob OID was correctly excluded. The phase-2 comment claims pathless objects are denied, but the allow filter does the opposite for trees on public repos. Fail closed on empty tree paths, or give every reachable tree a real path (recursive ls-tree walk) before the allow filter runs. Add a regression that asserts HEAD:secret/nested (or equivalent) never reaches a public pin backend in the sweep integration test; blob-only assertions do not cover this class.

• [P2] Align the phase-2 comment with the allow filter
  crates/gitlawb-node/src/git/visibility_pack.rs:581

  The comment at lines 581-583 says pathless cat-file objects are denied by the allow filter. For trees on public repos they are allowed when no glob matches "". Either implement deny-on-empty-path or rewrite the comment so the next reader does not treat pathless trees as already safe.

One process note, not a finding: expect rebase friction after #173, #324, and #330 land; those three share semantic surfaces with this diff.

Not an ask, recorded only: prior-round P2 items on per-repo OID fairness (reconciliation.rs:289) and preparation-phase fence coverage (ipfs_pin.rs) remain reasonable follow-ups once the tree classifier is fixed; I am not blocking this round on them.

I found issues that need to be addressed before this is ready.

Merge readiness

GitHub reports the current head as mergeable against main, and all 12 captured checks pass. The PR remains blocked by requested changes.

Findings

• [P1] Require a proven visibility path before publishing an object
  crates/gitlawb-node/src/git/visibility_pack.rs:610
  The new classifier has two producers with different evidence. Phase 1 runs git ls-tree -rz, which recursively emits blobs with paths but does not emit tree records without -t. Phase 2 runs cat-file --batch-all-objects, which includes the missing tree OIDs as well as dangling objects, but can attach no path and inserts them as (oid, ""). The allow loops then pass that empty string to visibility_check; on a public repository it matches no scoped rule and falls through to the repository-wide default Allow. As a result, a tree under a denied /secret/** scope enters allowed_trees and can be uploaded by both reconciliation backends. The receive-pack full-scan path has an additional exposure: unlike the sweep, it does not intersect the result with reachable_object_oids, so a dangling blob admitted through the same empty-path fallback can also be published in cleartext.

  The root problem is that “no path evidence” is represented as an ordinary path and sent through a predicate whose no-match behavior is allow. Make unknown/unreachable provenance a distinct fail-closed state: an object should enter a public allow-set only when the classifier has at least one proven reachable path that the current policy permits. Enumerate reachable tree paths deliberately (including an explicit policy for the root tree), and keep pathless batch-all-objects entries classified as unknown/denied rather than using the public default. Merely adding -t is incomplete because the root tree and dangling blobs still have no proven path; merely changing the comment leaves the leak intact. Add direct classifier tests plus sink-level negative tests showing that a denied nested tree never reaches IPFS or Pinata and that a dangling blob never survives the receive-pack full-scan fallback.

• [P2] Preserve a usable CID for Pinata-only rows in the pins contract
  crates/gitlawb-node/src/api/ipfs.rs:704
  This PR correctly separates backend provenance in storage: a Pinata-only row now has cid = NULL and pinata_cid = <remote CID>. The existing endpoint contract and client, however, still depend on the old compatibility shape where cid is always usable. The handler comment explicitly says that Pinata-only rows copy pinata_cid into the response-level cid, but the manual JSON mapping serializes p.cid directly. gl ipfs list reads only pin["cid"], so every remote-only row is counted but rendered as ?; at the merge base record_pinata_cid populated the non-null cid column with the Pinata CID, so this response regression did not exist.

  Fix the contract boundary rather than undoing the new database model. Either preserve backward compatibility by serializing cid as the local CID with a pinata_cid fallback while retaining both provenance fields, or intentionally define/version a nullable response and update every consumer in the same change. If the endpoint lists both backends, update the CLI labels so a Pinata CID is not presented as a local-daemon pin. Add exact endpoint and CLI cases for local-only, Pinata-only, dual-backend, and invalid neither-backend rows; the Pinata-only case must assert the displayed CID rather than only the row count.

Needs maintainer decision

• Decide whether failed Irys anchors are one-shot or retryable
  crates/gitlawb-node/src/reconciliation.rs:885
  The PR notes describe encrypted anchoring as best effort, but the failure log says it “will retry next pass.” It cannot currently do that: each envelope and recipient tag is persisted before the anchor call, and the next pass treats that stored tag as completed work. plan_seal returns SkipUnchanged, sealed is empty, and the !sealed.is_empty() gate prevents another anchor attempt. The same gap exists across a process exit after the encrypted-row write and before the Irys acknowledgement.

  Please choose and encode one contract. If anchoring is intentionally one-shot best effort, remove the retry claim and document that a transient Irys failure can leave the delta unanchored. If configured anchoring is part of the recovery guarantee, give it its own durable lifecycle: persist a pending manifest/outbox record independently of the seal decision, mark it anchored only after acknowledgement, and retry pending records after restart without resealing unchanged blobs. Key the work idempotently so a timeout after remote success can be retried without creating uncontrolled duplicate manifests, and test both the transient-failure and crash-between-persist-and-anchor boundaries.

Guidance before the next review

The repeated review rounds are coming from one underlying pattern: this PR crosses several security and durability contracts, but those contracts are still encoded indirectly in local predicates, comments, nullable fields, and “already done” checks. A local repair can make one call site look correct while changing the meaning expected by the next stage. The tree fix illustrates this: adding a second object producer made the inventory more complete, but represented missing provenance as ""; the existing visibility predicate then interpreted that sentinel through its normal public fallback. The Pinata fix has the same shape at a different boundary: storage now represents provenance accurately, but the established wire/client compatibility contract was not updated with it. More patching at individual symptoms is likely to create another round unless the end-to-end contracts are made explicit first.

Before changing more code, write down a small invariant matrix and use it as the implementation and test plan. For public replication, the rows should cover at least commits, tags, root trees, nested trees, blobs, objects reachable at multiple paths, objects reachable only from non-commit refs, and dangling/unreachable objects. The columns should state: what producer discovers the object; what evidence proves reachability and path provenance; how root and path-scoped rules apply; whether an empty/unknown path is ever valid; and whether the object may reach IPFS, Pinata, the encrypted sink, or no sink. Make the default explicit: an object with incomplete classification or no proven allowed path does not enter a public sink. Also state the intended tree-metadata policy—including how a tree containing denied descendants is handled—so the implementation is not forced to infer that policy from blob behavior.

Then make that matrix one source of truth in code. Avoid using strings such as "" to carry security-relevant state; use a representation that distinguishes a proven path from unknown provenance, and make the public-allow operation impossible or explicitly fail closed for the unknown variant. Keep reachable inventory, path classification, and policy evaluation together enough that adding a broader producer cannot silently bypass the classifier. Both the receive-pack full-scan fallback and reconciliation should consume the same final publication decision rather than applying slightly different cleanup afterward. The sweep's extra reachable intersection is useful defense in depth, but it should not be the reason one caller is safe while the shared classifier remains unsafe for another.

Model durability work as explicit backend obligations rather than inferring completion from whichever row happens to exist. For each enabled phase, identify the durable transition and its retry owner: object discovered → backend write required → backend acknowledged → local state persisted; and, if Irys retry is required, manifest pending → remote acknowledgement → anchored. For every transition, decide what happens on timeout, database failure, shutdown, and process exit between the external side effect and local persistence. Content addressing makes retries safe only when the local state machine retains enough information to know what remains pending; SkipUnchanged must not erase a downstream obligation that has not completed.

Treat the database model and public API as separate contracts. The nullable local CID is the right way to preserve backend provenance internally, but changing persistence does not automatically authorize changing an existing response field. Define the response matrix for local-only, Pinata-only, both, and neither, including the exact meaning and nullability of every field. Trace that matrix through all in-repository consumers—especially gl ipfs list—and either preserve compatibility or version the change deliberately. Tests should assert exact JSON and rendered user output, not only successful status or row count.

Finally, validate the negative property at the irreversible boundaries. Helper-level set assertions are necessary but insufficient for a confidentiality path: tests should observe the mock IPFS/Pinata endpoints and prove that denied/pathless/dangling bytes were never submitted. Cover the same matrix through both full-scan producers, not just reconciliation. For lifecycle behavior, use two-pass and restart-style tests that fail after each persistence boundary and then prove the next pass completes only the missing work. A focused end-to-end suite built from the invariant matrix will be more valuable than adding another test for each review comment in isolation.

Before requesting another round, please reconcile the PR description, code comments, and tests against that model in one pass. In particular, remove claims such as “pathless objects are denied” or “will retry next pass” unless a test demonstrates the exact sink/restart behavior. A concise matrix in the PR description plus the corresponding negative sink and failure-boundary tests would give reviewers one stable contract to verify and should prevent another sequence of narrowly discovered follow-ups.

I found issues that need to be addressed before this is ready.

Merge readiness

• [P1] Rebase onto current main and resolve the conflicting changed surfaces
  crates/gitlawb-node/src/db/mod.rs:883
  GitHub reports this head as CONFLICTING (DIRTY) against the live target, and the captured merge-tree has conflicts in files this PR changes. Please rebase onto current main and have the resolved diff re-reviewed so the migration, visibility, and replication changes are evaluated together.

Findings

• [P1] Preserve allowed tree objects in the full-scan replication set
  crates/gitlawb-node/src/git/visibility_pack.rs:542
  The new tree-aware full scan calls git ls-tree -rz. -r recursively emits blobs, but without -t Git does not emit the tree entries themselves. The later cat-file --batch-all-objects pass then discovers those tree OIDs without path provenance and inserts them with an empty path. The new fail-closed check deliberately excludes empty paths from allowed_trees, so replicable_objects_fail_closed removes every tree from the candidate set.

  This is not limited to an unusual repository shape: any reachable commit with a directory has at least its root tree, and nested directories add further trees. The reconciliation worker obtains its candidates from the full object database and passes them through this filter; the changed push-side full-scan fallback uses the same helper. It can therefore pin commits and allowed blobs while omitting the trees that connect commits to those blobs. A repaired backend no longer contains a complete reachable Git object graph, defeating the durability backstop this PR adds.

  Please address the root cause rather than weakening the empty-path guard: make the path-provenance walk represent every reachable tree that may pass the full-scan filter, including the root tree and nested trees, and then evaluate those paths under the same visibility rules as blobs. ls-tree options may be part of that solution, but ensure the resulting representation includes the tree invoked at the root as well as child trees; simply allowing pathless cat-file entries would reintroduce the unknown-provenance publication risk this change was meant to close. Add regression coverage for a public repository with nested directories and for path-scoped denied subtrees, proving that allowed root/nested trees remain replicable while denied or unclassifiable trees do not.

Review guidance

This PR has attracted repeated feedback because it changes one durability contract across several tightly coupled boundaries: Git object discovery and reachability, path-scoped visibility, irreversible public replication, encryption for withheld content, database provenance, cursor-based recovery, and concurrent policy changes. A local fix at one stage can silently invalidate an invariant at a later stage—for example, a fail-closed provenance guard is correct only if the discovery stage supplies provenance for every object type that the replication stage needs.

For the rebase and follow-up, please treat the sweep as an end-to-end object-graph contract rather than a collection of independent hardening changes. For each candidate object type (commit, tree, blob, tag, and unknown/dangling object), trace: enumerate → establish reachability → attach path/visibility provenance where applicable → authorize/refilter at dispatch → upload or encrypt → persist backend state → re-derive it on the next sweep. Tests should exercise that complete path with nested public content, a withheld subtree, objects reachable only through history, and an unclassifiable/dangling object. The expected split is narrow: retain all objects needed for an authorized reachable Git graph, deny objects whose path/visibility cannot be proven, and never make the latter exception just to recover the former.

Keeping that matrix explicit in the implementation and tests will make the remaining behavior reviewable after the required rebase and should avoid another sequence of narrowly discovered regressions.

Re-reviewed head e54c3b8 by execution after jatmn's round on the same head. I confirmed his tree-classifier finding with a local git ls-tree -rz fixture: without -t it emits blobs only, no tree lines. test (stable) is still red on this head; sweep_never_pins_withheld_blob_in_cleartext passes locally. Dismissed my stale CHANGES_REQUESTED on c2d690b4.

Findings

• [P1] Rebase onto current main before the next round
  crates/gitlawb-node/src/db/mod.rs:883
  GitHub reports this head as CONFLICTING against live main, and nine overlap files include db/mod.rs, api/repos.rs, and ipfs_pin.rs. The migration, visibility, and replication changes need to be reviewed on the resolved diff, not this stale merge base.

• [P1] Enumerate reachable trees with proven paths before the fail-closed filter
  crates/gitlawb-node/src/git/visibility_pack.rs:542
  Phase 1 runs git ls-tree -rz per commit, which lists blob paths but not tree OIDs (-t is absent). Phase 2 adds tree OIDs from cat-file --batch-all-objects with an empty path. The allow loop now skips path.is_empty(), so allowed_trees stays empty and replicable_objects_fail_closed drops every tree from the candidate set. Any repo with directories loses the trees that connect commits to allowed blobs, so the sweep can pin commits and blobs without restoring a complete reachable object graph. Fix the discovery stage (explicit tree walk that includes the root tree and nested trees, with real paths) rather than weakening the empty-path guard. Add regression coverage for nested public directories and for a denied subtree, proving allowed trees replicate and denied or unclassifiable trees do not.

• [P1] Bound receive-pack full-scan candidates to ref-reachable objects
  crates/gitlawb-node/src/api/repos.rs:126
fail_closed_full_scan_objects shares the classifier above but never calls reachable_object_oids, unlike the sweep at reconciliation.rs:465-478. push_delta.rs:215-222 documents why: dangling commits have no path to fail closed against, so replicable_objects_fail_closed passes them through as structural metadata. Empty-path denial fixes pathless blobs and trees but not dangling commits on the receive-pack full-scan fallback. Extract the shared reachability filter both call sites consume.

• [P2] Get test (stable) green on this head
  crates/gitlawb-node/src/api/repos.rs:9503
  Run 32816576285 fails f2a_first_push_is_admitted_and_does_the_full_work with try_recv() empty after branch-to-CID mapping succeeded. The test passes locally in isolation. The GraphQL broadcast is gated on announce inside the detached Pinata worker while mapping upsert is not; harden with a bounded recv() poll or trace why announce is false under CI load.

Not an ask, recorded only: the Pinata-only list_pins effective_cid fallback on this head looks correct. The Irys warn at reconciliation.rs:915 still promises a retry that SkipUnchanged prevents; pick one-shot best effort or a pending-manifest outbox when you touch that path again.